Skip to content

adjusts js winml sdk to depend on standard sdk and replace dependencies with winml binaries#596

Open
prathikr wants to merge 9 commits intomainfrom
prathikrao/js-sdk-import-fix
Open

adjusts js winml sdk to depend on standard sdk and replace dependencies with winml binaries#596
prathikr wants to merge 9 commits intomainfrom
prathikrao/js-sdk-import-fix

Conversation

@prathikr
Copy link
Copy Markdown
Contributor

@prathikr prathikr commented Apr 5, 2026

  • allows users to do npm install foundry-local-sdk-winml and then do import { FoundryLocalManager } from 'foundry-local-sdk'; like the other SDKs do
  • creates a compute version stage at the beginning of mega pipeline so all artifacts are generated with the same version string down to the timestamp second

Copilot AI review requested due to automatic review settings April 5, 2026 18:33
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
foundry-local Ready Ready Preview, Comment Apr 6, 2026 5:29pm

Request Review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restructures the JS WinML variant into a thin wrapper package (foundry-local-sdk-winml) that depends on foundry-local-sdk, intending to let users install the WinML variant but continue importing from foundry-local-sdk while swapping in WinML native binaries.

Changes:

  • Updates the packaging script to publish foundry-local-sdk-winml as a wrapper that depends on foundry-local-sdk and does not ship dist/.
  • Updates the WinML install script to target the standard SDK’s installation directory for native binary placement.
  • Extends the shared install utilities to support installing binaries into an explicit SDK root (rather than only this package’s own directory).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
sdk/js/script/pack.cjs Converts the WinML package into a thin wrapper (no dist/, depends on foundry-local-sdk).
sdk/js/script/install-winml.cjs Attempts to run preinstall + download WinML-native binaries into the standard SDK’s platform directory.
sdk/js/script/install-utils.cjs Adds sdkRoot support to direct extraction to a different @foundry-local-core/<platform> directory.
Comments suppressed due to low confidence (1)

sdk/js/script/install-utils.cjs:180

  • runInstall() returns early if the required files already exist and npm_config_nightly isn’t set. For the WinML wrapper use-case (overwriting an existing standard install), this prevents the WinML artifacts from ever being applied. Add an explicit force/overwrite option (or a check that detects whether the existing Core is the WinML variant) and use it from install-winml.cjs.
async function runInstall(artifacts, sdkRoot) {
    if (!RID) {
        console.warn(`[foundry-local] Unsupported platform: ${platformKey}. Skipping.`);
        return;
    }

    const binDir = sdkRoot
        ? path.join(sdkRoot, 'packages', '@foundry-local-core', platformKey)
        : DEFAULT_BIN_DIR;

    if (fs.existsSync(binDir) && REQUIRED_FILES.every(f => fs.existsSync(path.join(binDir, f)))) {
        if (process.env.npm_config_nightly === 'true') {
            console.log(`[foundry-local] Nightly requested. Forcing reinstall...`);
            fs.rmSync(binDir, { recursive: true, force: true });
        } else {
            console.log(`[foundry-local] Native libraries already installed.`);
            return;
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@prathikr prathikr changed the base branch from main to prathikrao/update-versions April 5, 2026 18:37
@prathikr prathikr changed the base branch from prathikrao/update-versions to main April 5, 2026 18:38
baijumeswani
baijumeswani previously approved these changes Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants